Hmm, looks like eslint needs to be upgraded before it'll parse the using test

Not sure about destroyOnDispose, i.e. whether its job is common enough to justify it and whether it’s the best API for the job. Would suggest initializing to false if kept. Note .release(true) continues to work for error cases given that double release is a no-op; I recognize that it doesn’t satisfy the use case you mentioned. Maybe release the feature without that API initially, since it can be added easily?

Yeah I wasn't sure about the API either but I couldn't think of a better way to do it. Maybe a client.setError() to set an error state that client.release() acknowledges as well?

That said, unfortunately I don't think relying on double-release is workable either, because the second release, which will be called from within Symbol.dispose will throw. Even if [Symbol.dispose]() wraps that in a try-catch to swallow the error, needing to put the client.release(true) in a finally clause obviates any purpose of using the using syntax.

I got here because I'm implementing dispose on a client wrapper.

One question, why implement destroyOnDispose as a mutable property instead of an internal one initialized by pool.connect()? e.g.

using client = await pool.connect({ destroyOnDispose: true });

Maybe I'm not understanding the use case you mention.

@olsonpm That does work for my usecase, I just didn't think of it.

But on the other hand it's still a shift from how late you can make the decision not to return the client to the pool -- with client.release(true), you're able to make this decision all the way at the end of your session (starting from pool.connect until client.release) with this client, but with a pool.connect parameter, you need to commit to destroying the client all the way at the beginning of your session, before you even have the client instance.

An example where it might be desirable to destroy the client rather than release it back to the pool would be if you made a client.query and caught an error that indicated that the client was no longer usable. You might then want to set destroyOnDispose = true instead of letting the client go back to the pool.

Ah thanks for that use case Hyperair. I was aware the decision would change the semantics as you mentioned, but couldn't think of why you would want that flexibility.

Hey thanks for this! I actually set out to do this a while ago and due to the linting and it not working on older versions of node I grew frustrated and thought I'd do it later. And now it's done!

I'm also not sold on destroyOnDispose though...it is kinda a weird API to me. I can't really think of a better way to handle it though. I found it kinda frustrating actually in the dispose ecmascript design there's no way to know if an error was thrown in the scope or if the scope exited "naturally." I took a stab at having a using client.transaction() type thing but not being able to issue a rollback on error was a show stopper.

One possible alternative is to just allow ended clients to be returned to the pool (via the disposal or via client.release()) and if the pooled client has had 'end' called then it is evicted automatically from the pool. This would prevent likely error scenarios of misuse anyway where one accidentally returns an ended client (obviously not recommended, but could happen) and then the code would look like this

using client = await pool.connect()
try {
  client.query("EXPLODE!")
} catch (e) {
  client.end()
  throw e // (or just log it if you want or whatever)
}

Thoughts on that?